fix: honor connection conditions during provisioning - #9678
Open
Hui Miao (huimiu) wants to merge 9 commits into
Open
fix: honor connection conditions during provisioning#9678Hui Miao (huimiu) wants to merge 9 commits into
Hui Miao (huimiu) wants to merge 9 commits into
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 18 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
ext-agents
Hui Miao (huimiu)
marked this pull request as ready for review
August 21, 2026 14:14
|
Azure Pipelines: Successfully started running 2 pipeline(s). 19 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures Foundry connection conditions are honored consistently during synthesis and provisioning.
Changes:
- Filters disabled connections before
$refand variable expansion. - Evaluates conditions using resolved project environments.
- Adds coverage for disabled, referenced, whitespace, and brownfield connections.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
azure.ai.projects/internal/synthesis/synthesizer.go |
Filters connection synthesis and scopes by condition. |
azure.ai.projects/internal/synthesis/synthesizer_test.go |
Tests Projects condition behavior. |
azure.ai.projects/internal/synthesis/condition.go |
Adds condition evaluation helpers. |
azure.ai.projects/internal/provisioning/foundry_provisioning_provider.go |
Resolves environment before discovering scopes. |
azure.ai.agents/internal/synthesis/synthesizer.go |
Mirrors connection filtering for Agents. |
azure.ai.agents/internal/synthesis/synthesizer_test.go |
Tests Agents condition behavior. |
azure.ai.agents/internal/synthesis/condition.go |
Mirrors condition evaluation helpers. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Hui Miao (huimiu)
requested review from
Marina He (hemarina),
tg-msft and
Victor Vazquez (vhvb1989)
as code owners
August 21, 2026 14:49
Hui Miao (huimiu)
requested review from
Rick Winter (RickWinter) and
Richard Park (richardpark-msft)
as code owners
August 21, 2026 14:49
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/synthesis/condition.go:35
azd-code-reviewer: Numeric conversion changes the condition’s lexical value before the exact truthiness check. Core decodesconditionthroughExpandableStringand compares the original scalar text, so values such ascondition: 0x1orcondition: 1.0are disabled there; this implementation normalizes them to"1"and enables the connection during synthesis. That can reintroduce a connection that core filtered out. Decode the YAML scalar as a string (preserving its text) and apply the same exact comparison asServiceConfig.IsEnabled.
case int:
return isTruthyCondition(strconv.Itoa(v)), nil
case int8:
return isTruthyCondition(strconv.Itoa(int(v))), nil
cli/azd/extensions/azure.ai.projects/internal/synthesis/condition.go:35
azd-code-reviewer: Numeric conversion changes the condition’s lexical value before the exact truthiness check. Core decodesconditionthroughExpandableStringand compares the original scalar text, so values such ascondition: 0x1orcondition: 1.0are disabled there; this implementation normalizes them to"1"and enables the connection during synthesis. That can reintroduce a connection that core filtered out. Decode the YAML scalar as a string (preserving its text) and apply the same exact comparison asServiceConfig.IsEnabled.
case int:
return isTruthyCondition(strconv.Itoa(v)), nil
case int8:
return isTruthyCondition(strconv.Itoa(int(v))), nil
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
cli/azd/extensions/azure.ai.projects/internal/synthesis/condition.go:25
- [azd-code-reviewer]
foundry.ExpandEnvdeliberately preserves${{...}}, so this does not actually matchServiceConfig.IsEnabled: core'sExpandableString.Envsubstrejects a condition such as${{event.body}}as malformed, while synthesis leaves it unchanged and silently treats the connection as disabled. Use the sameosutilexpander as core so validation and selection cannot disagree.
expanded, err := foundry.ExpandEnv(value, getenv)
cli/azd/extensions/azure.ai.agents/internal/synthesis/condition.go:25
- [azd-code-reviewer]
foundry.ExpandEnvdeliberately preserves${{...}}, so this does not actually matchServiceConfig.IsEnabled: core'sExpandableString.Envsubstrejects a condition such as${{event.body}}as malformed, while synthesis leaves it unchanged and silently treats the connection as disabled. Use the sameosutilexpander as core so validation and selection cannot disagree.
expanded, err := foundry.ExpandEnv(value, getenv)
Contributor
azure.ai.projects PR buildNote This is an unsigned development build. Install it only if you trust this PR. Install the extension: azd ext install "https://azuresdkartifacts.z5.web.core.windows.net/azd/extensions/pr/9678/azure-ai-projects.zip"
|
Contributor
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Travis Angevine (trangevi)
approved these changes
Aug 21, 2026
Travis Angevine (trangevi)
left a comment
Travis Angevine (trangevi)
left a comment
Member
There was a problem hiding this comment.
Approving for extension changes
JeffreyCA
reviewed
Aug 25, 2026
| // handlers to participate in the lifecycle of an azd project | ||
| Initialize(ctx context.Context, projectConfig *ProjectConfig) error | ||
|
|
||
| // InitializeServices initializes the supplied services. |
Contributor
There was a problem hiding this comment.
[suggestion] InitializeServices does not evaluate service conditions. It initializes every service in the supplied slice. Please state that the caller must apply condition and command filters first. Otherwise, a future caller can reasonably read this as a condition-aware replacement for Initialize and reintroduce the ordering bug this PR fixes.
| }) | ||
| } | ||
|
|
||
| func Test_projectManager_InitializeServices_RespectsRootConditions(t *testing.T) { |
Contributor
There was a problem hiding this comment.
[suggestion] This test name assigns condition handling to InitializeServices, but the test applies ServiceStableFiltered before calling that method. Please rename this to describe initialization of the supplied slice, and keep the condition-ordering assertion at the command level where that contract is implemented.
| Module: "main", | ||
| }, | ||
| } | ||
| connection := &project.ServiceConfig{ |
Contributor
There was a problem hiding this comment.
[suggestion] This checks that ProvisionAction passes an empty slice to a mocked ProjectManager, but the missing $ref can never be resolved through this mock. Please add coverage with a service target that fails if initialized, or an equivalent functional test, so the original azd provision --no-prompt failure is protected at the actual service initialization boundary.
| return nil, err | ||
| } | ||
|
|
||
| if err := p.projectManager.EnsureAllTools(ctx, p.projectConfig, selectedServiceFilter(services)); err != nil { |
Contributor
There was a problem hiding this comment.
[suggestion] We already have the exact selected service slice here, but EnsureAllTools re-enumerates the project and reconstructs that selection by name. Consider passing the selected services directly into tool discovery, or add an API that accepts this slice. This keeps condition evaluation as one command-level snapshot and avoids running dynamic service import again against a potentially different set of ServiceConfig values.
JeffreyCA
reviewed
Aug 25, 2026
Contributor
There was a problem hiding this comment.
The core ordering is headed in the right direction, but I think we need to make the selected-service contract explicit and cover the reported non-preview failure at the real initialization boundary before merging.
-
Since the extension changes depend on new core behaviour, we should remember to update the extension's
requiredAzdVersion -
We should also update the remaining command paths separately (happy to take this as a follow-up): standalone
build,package,restore,publish, anddeploystill initialize services or inspect tools before applyingServiceStableFiltered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this is needed
azure.ai.connectionservices did not always honorconditionduring synthesis. A disabled connection could still appear in Foundry provisioning or on-disk environment scopes. Whitespace-only values are non-empty and disable the service, matchingpkg/project.ServiceConfig.IsEnabled.Why this approach
The Projects and Agents synthesis paths use the same condition rules. They check conditions before expanding connection payloads. A root
condition: falseskips a missing or broken service$ref. A condition found only in a referenced payload produces an error telling the author to put it besidehostinazure.yaml.Provisioning and
azd upnow select root-enabled services before initializing service targets or resolving their required tools. This prevents a disabled service from resolving its payload$ref, while keeping the existing service graph and active-environment condition evaluation. Explicit subscription and location options are applied before this selection.The provider resolves the environment before discovering connection scopes, so provisioning and synthesis use the same enabled connections. Payload expansion, secret handling, sorting, brownfield filtering, and provider timing stay unchanged.
Legacy manifest strictness and target/credentials/metadata variable validation remain out of scope for this PR.
E2E result
With a root-disabled
azure.ai.connectionservice whose$refpoints to a missing payload, it failed: service initialization resolved the missing$refbefore the Projects provider could filter the disabled connection. The current core change addresses that path; the E2E run has not been repeated against the new head.The same E2E run passed
azd provision --preview --no-promptfor the root-disabled missing-ref scenario, and passed Agent Bicep and Terraform eject scenarios for disabled missing refs and whitespace conditions.Closes: #9686